Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[generator] fixed new warnings introduced #224

Merged
merged 1 commit into from
Dec 14, 2017

Conversation

jonathanpeppers
Copy link
Member

Context: #217

When testing generator from xamarin-android/master, I noticed that
building a binding project has some new warnings:

obj/Debug/generated/src/GoogleAnalytics.Tracking.ILogger.cs(159,14): warning CS0109:
	The member 'ILoggerInvoker.class_ref' does not hide an accessible member.
	The new keyword is not required.

Binding project: https://github.com/jonathanpeppers/GoogleAnalytics/tree/master/GoogleAnalytics.Droid

This was due to a change made in #217, I was tricked by how warnings
operate differently in generator-Tests than what happens in a real
Xamarin.Android binding project.

The difference is:

  • generator-Tests put everything in one assembly
  • Real binding projects have Java.Lang.* and Android types in a
    separate assembly

This means that some members that are marked internal will not need
the new keyword at all—even though the tests currently would need
the new keyword. The real fix here is to make the tests generate two
separate assemblies so that we are more closely reproducing the
situation of a Xamarin.Android binding project.

Since we are branching for 15-6 soon, it seems better to revert the
new keyword change in InterfaceGen, and set AllowWarnings=true
where required in generator-Tests..

I also included a comment for every test that sets
AllowWarnings=true, so that it is clear which warning is occurring.

Context: dotnet#217

When testing generator from xamarin-android/master, I noticed that
building a binding project has some *new* warnings:
```
obj/Debug/generated/src/GoogleAnalytics.Tracking.ILogger.cs(159,14): warning CS0109:
	The member 'ILoggerInvoker.class_ref' does not hide an accessible member.
	The new keyword is not required.
```
Binding project: https://github.com/jonathanpeppers/GoogleAnalytics/tree/master/GoogleAnalytics.Droid

This was due to a change made in dotnet#217, I was *tricked* by how warnings
operate differently in `generator-Tests` than what happens in a real
Xamarin.Android binding project.

The difference is:
- `generator-Tests` put everything in one assembly
- Real binding projects have `Java.Lang.*` and Android types in a
separate assembly

This means that some members that are marked `internal` will not need
the `new` keyword at all—even though the tests currently *would* need
the `new` keyword. The real fix here is to make the tests generate two
separate assemblies so that we are more closely reproducing the
situation of a Xamarin.Android binding project.

Since we are branching for 15-6 soon, it seems better to revert the
`new` keyword change in `InterfaceGen`, and set `AllowWarnings=true`
where required in `generator-Tests`..

I also included a comment for every test that sets
`AllowWarnings=true`, so that it is clear which warning is occurring.
@jonpryor jonpryor merged commit 8da427b into dotnet:master Dec 14, 2017
jonpryor pushed a commit that referenced this pull request Jan 26, 2024
Changes: dotnet/android-tools@4889bf0...ed102fc

  * dotnet/android-tools@ed102fc: [Xamarin.Android.Tools.Versions] Add JavaSdkVersion (#226)
  * dotnet/android-tools@b175674: [ci] Only enable CodeQL on Windows build job (#224)
  * dotnet/android-tools@2a2e64b: [ci] Add API Scan job (#225)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@github-actions github-actions bot locked and limited conversation to collaborators Apr 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants